fix(install): name the non-interactive trigger in the --station-deepseek conflict error (#7009) - #7013
Closed
yanyunl1991 wants to merge 1 commit into
Closed
fix(install): name the non-interactive trigger in the --station-deepseek conflict error (#7009)#7013yanyunl1991 wants to merge 1 commit into
yanyunl1991 wants to merge 1 commit into
Conversation
…eek conflict error (#7009) When --station-deepseek conflicts with non-interactive mode, the error printed an identical message no matter what caused it, so users could not tell why it was rejected. main() exports NON_INTERACTIVE into NEMOCLAW_NON_INTERACTIVE, so the origin cannot be recovered at error time. Record the trigger during flag parsing and name it in the message: ... cannot be combined with non-interactive mode (triggered by: the --non-interactive flag). ... cannot be combined with non-interactive mode (triggered by: NEMOCLAW_NON_INTERACTIVE=1). The "(triggered by: ...)" clause is only appended when the origin is known, so direct callers that set NON_INTERACTIVE without going through flag parsing still get a clean message. Companion to #7008 (which stops notice acceptance from implying non-interactive for --station-deepseek); kept as a separate PR per issue. This PR touches only the trigger tracking and the error message, not the decouple, so the two do not overlap. Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Contributor
📝 WalkthroughWalkthroughThe installer now records whether non-interactive mode came from the command-line flag or environment and includes that source in Station DeepSeek conflict errors. Tests cover both trigger sources and generic non-interactive mode messaging. ChangesStation DeepSeek trigger reporting
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/install-express-prompt.test.ts`:
- Line 369: Strengthen the fallback assertion in the NON_INTERACTIVE case of the
install-express prompt test by anchoring the expected message ending or matching
the complete message, ensuring it cannot match text containing a “(triggered by:
…)” suffix. Preserve the test’s validation of the direct-caller fallback
contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7d901185-b4db-4b0d-acef-cb33be462d7d
📒 Files selected for processing (2)
scripts/install.shtest/install-express-prompt.test.ts
| it.each([ | ||
| ["NEMOCLAW_NO_EXPRESS", "1", /cannot be combined with NEMOCLAW_NO_EXPRESS=1/], | ||
| ["NON_INTERACTIVE", "1", /cannot be combined with --non-interactive/], | ||
| ["NON_INTERACTIVE", "1", /cannot be combined with non-interactive mode/], |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the fallback assertion prove that no trigger is included.
The current unanchored regex matches both the generic message and a message containing (triggered by: ...). Anchor the expected ending or assert the complete message so this test protects the direct-caller fallback contract.
Suggested fix
- ["NON_INTERACTIVE", "1", /cannot be combined with non-interactive mode/],
+ ["NON_INTERACTIVE", "1", /cannot be combined with non-interactive mode\.$/],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ["NON_INTERACTIVE", "1", /cannot be combined with non-interactive mode/], | |
| ["NON_INTERACTIVE", "1", /cannot be combined with non-interactive mode\.$/], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/install-express-prompt.test.ts` at line 369, Strengthen the fallback
assertion in the NON_INTERACTIVE case of the install-express prompt test by
anchoring the expected message ending or matching the complete message, ensuring
it cannot match text containing a “(triggered by: …)” suffix. Preserve the
test’s validation of the direct-caller fallback contract.
Source: Path instructions
Contributor
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: Blockers
|
Contributor
Author
This was referenced Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
--station-deepseekconflicts with non-interactive mode, the installer printed the same error regardless of what caused it, so users following the documented install could not tell why the flag was rejected:main()exportsNON_INTERACTIVEintoNEMOCLAW_NON_INTERACTIVE, so the origin cannot be recovered at error time.Fix
Record the trigger during flag parsing and name it in the message:
The
(triggered by: …)clause is appended only when the origin is known, so direct callers that setNON_INTERACTIVEwithout going through flag parsing still get a clean message.Verification (DGX-Station-mocked aarch64)
--non-interactive+--station-deepseek→ names the --non-interactive flag.NEMOCLAW_NON_INTERACTIVE=1+--station-deepseek→ names NEMOCLAW_NON_INTERACTIVE=1.Updated/added cases in
test/install-express-prompt.test.ts(the harness sources the real installer, so these run the actual shell logic).Relationship to #7008
Companion to #7008, which stops
NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1from implying non-interactive for--station-deepseek. Kept as a separate PR per issue. This PR touches only the trigger tracking and the error message (flag parsing + the validator), not the accept-notice block that #7008 changes, so the two PRs do not overlap and can merge in either order. Once #7008 lands, notice acceptance is no longer a trigger here, and the remaining triggers (the flag and env var) are both named.Fixes #7009.
Signed-off-by: Yanyun Liao yanyunl@nvidia.com
🤖 Generated with Claude Code
Summary by CodeRabbit